Skip to content

Conversation

@Kobzol
Copy link
Member

@Kobzol Kobzol commented Jan 16, 2026

Forgot about this in #145354.

Created a tracking issue in #151364.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 16, 2026
@rustbot
Copy link
Collaborator

rustbot commented Jan 16, 2026

r? @jackh726

rustbot has assigned @jackh726.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@jplatte
Copy link
Contributor

jplatte commented Jan 16, 2026

Is there a tracking issue?

@Urgau
Copy link
Member

Urgau commented Jan 16, 2026

Is there a tracking issue?

If there isn't one, we should create one and link it from the new page.

@@ -0,0 +1,3 @@
## `cache-proc-macros`

This option instructs `rustc` to cache (derive) proc-macro invocations using the incremental system. Note that this can be unsouned - we currently do not check if the macro invocation is actually "cacheable" or not.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This option instructs `rustc` to cache (derive) proc-macro invocations using the incremental system. Note that this can be unsouned - we currently do not check if the macro invocation is actually "cacheable" or not.
This option instructs `rustc` to cache (derive) proc-macro invocations using the incremental system. Note that this can be unsound - we currently do not check if the macro invocation is actually "cacheable" or not.

also, about the "we currently do not check" part, is the plan is to infer whether the proc-macro is cacheable (seems unlikely), or rather respect what the user promises about its behavior? This could just mention that enabling the flag for proc-macros that are not pure functions is bound to reuse stale extensions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather respect what the user promises about its behavior?

That is the plan, I suppose? There were a lot of proposals for this, but I don't think we have a winner :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT

@Kobzol Kobzol force-pushed the document-cache-proc-macros branch from d552bac to 345e9a0 Compare January 19, 2026 11:42
@Kobzol
Copy link
Member Author

Kobzol commented Jan 19, 2026

Created a tracking issue, mentioned it in the docs and reworded the cacheability aspect.


------------------------

This option instructs `rustc` to cache (derive) proc-macro invocations using the incremental system. Note that the compiler does not currently check whether the proc-macro is actually "cacheable" or not. If you use this flag when compiling a crate that uses non-pure proc-macros, it can result in stale expansions being compiled.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the compiler does not currently check whether the proc-macro is actually "cacheable" or not.

Oh I sure hope we issue some loud warnings about potential unsoundness XD

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, currently we can't, unless we want to issue a loud warning everytime the flag is used.. :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I had in mind.

@jackh726
Copy link
Member

@bors r+ rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 23, 2026

📌 Commit 345e9a0 has been approved by jackh726

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 23, 2026
@jackh726
Copy link
Member

Actually @bors r-

I see you had wording specifically about unsoundness in the text prior. Why did you reword that? I think we should keep it.

@rust-bors rust-bors bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 23, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 23, 2026

Commit 345e9a0 has been unapproved.

@jackh726
Copy link
Member

But, not too invested in it, so r=me if you think strongly otherwise.

@Kobzol
Copy link
Member Author

Kobzol commented Jan 23, 2026

I mostly removed it by accident, but actually now that you mention it, I don't know if skipping recomputation of a proc macro can cause actual unsoundness. We will always recompute the macro if the input tokenstream (or the proc macro crate) changes. We can only skip recompilation if something external (e.g. a DB for sqlx :) ) changes. Unless the proc macro is reading other source code from the crate, I'm not sure if that can actually be unsound? It can certainly cause some bugs though..

@jackh726
Copy link
Member

It can be very true that something external to the proc macro changes, and the proc macro does some unsafe things that assume something like type layout or something.

@jplatte
Copy link
Contributor

jplatte commented Jan 23, 2026

I think such macros should already be considered unsound, since Rust makes no guarantees about when a macro gets (re)evaluated, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants